ENH: Make default-constructors of RGBPixel and RGBAPixel constexpr#5087
Conversation
github-actions
bot
added
type:Enhancement
I did this pull request by a regular |
|
I use TortoiseGit, which implicitly uses plain push. I forgot what review-push does, but I am sure it could be found in the documentation. |
N-Dekker
force-pushed
the
constexpr-default-constructors-RGBPixel-RGBAPixel
branch
from
7544cbd to
6464b88
Compare
| #else | ||
| RGBAPixel() { this->Fill(0); } | ||
| constexpr RGBAPixel() | ||
| : Superclass() |
There was a problem hiding this comment.
With this force-push, replacing {} with (), I'm hoping to address the warnings at https://open.cdash.org/viewBuildError.php?type=1&buildid=10096694 saying:
In file included from Modules/Core/Common/test/itkNumericTraitsTest.cxx:42:
/.../s/Modules/Core/Common/include/itkNumericTraitsRGBPixel.h: In function 'void numeric_traits_test::CheckVariableLengthArrayTraits(const T&) [with T = itk::RGBPixel<char>]':
Modules/Core/Common/include/itkNumericTraitsRGBPixel.h:118:17: warning: '<anonymous>' may be used uninitialized in this function [-Wmaybe-uninitialized]
There was a problem hiding this comment.
It appears that with the old Linux GCC 9.4.0, the constexpr default-constructors that I wrote just don't properly zero-initialize the components, as they should. I'll think of another workaround...
Update: almost reproduced the old compiler bug at https://godbolt.org/z/rzdv3GGG4:
struct FixedArray
{
FixedArray() = default;
FixedArray(int);
int m_InternalArray[3];
};
struct RGBPixel: FixedArray
{
RGBPixel(): FixedArray() {}
};
int main()
{
const auto x = RGBPixel();
return x.m_InternalArray[0];
}
Saying:
warning: 'x.RGBPixel::<anonymous>.FixedArray::m_InternalArray[0]' is used uninitialized in this function [-Wuninitialized]
There was a problem hiding this comment.
Another attempt, initializing by the seemingly redundant expression Superclass(Superclass()): force-push
There was a problem hiding this comment.
Now might be a reasonable time to reconsider which compiler versions we support in ITK 6?
There was a problem hiding this comment.
@dzenanz It looks like this particular bug (as I reproduced with the example code at #5087 (comment)) is fixed with GCC 12.1 (which was released in May 2022): https://godbolt.org/z/of6Tdbzcj GCC 11 still appears to have the compiler bug. I'm not sure if we can already drop GCC 11 now. Ubuntu-22.04 still comes with GCC 11.4 🤷
Having said so, I think we have a workaround for this one now 😃
There was a problem hiding this comment.
My Linux workstation is on Ubuntu 22.04. So yes, we can't drop it yet 😄
|
@N-Dekker I only use |
N-Dekker
force-pushed
the
constexpr-default-constructors-RGBPixel-RGBAPixel
branch
from
6464b88 to
07ee85a
Compare
Tested by means of `CheckConstexprBeginAndEndOfContainer()`. Note that these default-constructors were already `constexpr` _implicitly_, as implied by `= default`, when `ITK_FUTURE_LEGACY_REMOVE` would be enabled. Used the expression `Superclass(Superclass())` as a workaround for a compiler bug from GCC 9.4.0, causing warnings like: In file included from itkNumericTraitsTest.cxx:42: itkNumericTraitsRGBPixel.h: In function 'CheckVariableLengthArrayTraits(const T&) [with T = itk::RGBPixel<char>]': itkNumericTraitsRGBPixel.h:118:17: warning: '<anonymous>' may be used uninitialized in this function [-Wmaybe-uninitialized]
|
I think that "".github/workflows/itk_dict.txt"" needs to have "Wmaybe" added. |
N-Dekker
force-pushed
the
constexpr-default-constructors-RGBPixel-RGBAPixel
branch
from
07ee85a to
f407ae5
Compare
github-actions
bot
added
the
type:Infrastructure
|
I created an issue to track how this may be done automatically: #5092 |
4 participants
Tested by means of
CheckConstexprBeginAndEndOfContainer(). Note that these default-constructors were alreadyconstexprimplicitly, as implied by= default, whenITK_FUTURE_LEGACY_REMOVEwould be enabled.